-
Notifications
You must be signed in to change notification settings - Fork 0
[setting/#3] 디자인 시스템 세팅 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
vvan2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엄청 빠르시네요 역시 King God 승재 더 지니어스..
꼼꼼하게 봤는데 컨벤션도 잘지켜줬고 누락된거 없이 행간,자간 배율도 잘 맞춰주셨네요.
깔끔하게 잘 작성했으니까 어푸푸! 고생했으
|
|
||
| @Composable | ||
| fun KieroTheme( | ||
| darkTheme: Boolean = isSystemInDarkTheme(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p2) 다크모드 지원안하니까 이 파라미터도 빼도 될듯용
| typography = defaultKieroTypography | ||
| ) { | ||
| MaterialTheme( | ||
| colorScheme = LightColorScheme, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
갓승재) LightTheme로 고정이니까, 명시적으로 적어준거 아주 좋은데요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3) 피그마 DS 보다가 생각난건데용 별건아니고...
보통 typography 에 행간,자간 별로 세분화되어있는 케이스가 많은데 지금 프로젝트에서는 각 font(SemiBold,Bold,Regular) 마다 행간, 자간이 하나로 통일된게 많아서 아래 처럼 상수로 빼줘도 좋을 것 같긴합니다.
ex) SemiBold 행간 130% -> 140% 으로 변경해주세요! 했을 때 일괄적으로 변경하기 쉬울 것 같아서 써봤어요. 안고쳐도 되고 그냥 참고만.. ㅎㅎ
private object TypographyDefaults {
val RegularLetterSpacing = (-0.005).em
val RegularLineHeight = 1.3.em
val SemiBoldLetterSpacing = (-0.01).em
val SemiBoldLineHeight = 1.4.em
val BoldLetterSpacing = (-0.01).em
val BoldLineHeight = 1.4.em
} body1 = TextStyle(
fontFamily = NotoSansFont.regular,
fontSize = 18.sp,
letterSpacing = TypographyDefaults.RegularLetterSpacing,
lineHeight = TypographyDefaults.RegularLineHeight,
),
//사용할 때 이런식으로?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가적으로 만약 어떻게 해도 뭔가 디자인과 안맞는다 싶으면 TextStyle의 파라미터 중 하나인
platformStyle = PlatformTextStyle(
includeFontPadding = false
),
을 통해 안드로이드 폰트 기본 패딩을 제거해보고 사용해보셔도 좋을 것 같아요 이거도 혹시나 필요하다면 이런게 있다 정도만 생각해셔도 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상수로 빼주니 깔끔하네요!
dmp100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
훌륭하네요 ! 수고많으셨습니다~~
sonms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿굿 깔끔하니 좋네요 ㅎㅎ 고생많으셨습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가적으로 만약 어떻게 해도 뭔가 디자인과 안맞는다 싶으면 TextStyle의 파라미터 중 하나인
platformStyle = PlatformTextStyle(
includeFontPadding = false
),
을 통해 안드로이드 폰트 기본 패딩을 제거해보고 사용해보셔도 좋을 것 같아요 이거도 혹시나 필요하다면 이런게 있다 정도만 생각해셔도 좋을 것 같아요!
ISSUE
❗ WORK DESCRIPTION
📢 TO REVIEWERS
사용예시
📸 SCREENSHOT